Create @shopify/polaris-migrator package
#6701
Conversation
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
3 times, most recently
from
3db0d76 to
c855b1c
Compare
sam-b-rose
changed the title
@shopify/polaris-migrator package
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
from
c855b1c to
aee2299
Compare
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
7 times, most recently
from
6474907 to
8315b33
Compare
sam-b-rose
removed
the
🤖Skip Changelog
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
from
8315b33 to
0c3aae5
Compare
|
/snapit |
|
🫰✨ Thanks @samrose3! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20220801193602yarn add @shopify/polaris@0.0.0-snapshot-release-20220801193602 |
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
from
0c3aae5 to
bee4966
Compare
|
/snapit |
|
🫰✨ Thanks @samrose3! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20220801200625yarn add @shopify/polaris@0.0.0-snapshot-release-20220801200625 |
sam-b-rose
force-pushed
the
add-polaris-migrator
branch
from
bee4966 to
18e114a
Compare
BPScott
left a comment
BPScott
left a comment
There was a problem hiding this comment.
Heck yeah I love me some codemods!
Dropped some comments around the place. Noteably I think it'd be very valuable to use postcss-value-parser to get a better AST for handling transforms of prop values.
| postcssPlugin: 'ReplaceSassSpacing', | ||
| Declaration(decl) { | ||
| decl.value = decl.value.replace( | ||
| createRegexFromMap(spacingMap), |
There was a problem hiding this comment.
I'm not a fan of the usage of regex here. It looks like it can match false positives like my-spacing(), and miss variations in whitespace/quoting like spacing( none ), spacing('none') and spacing( "none" ) (admittedly as we use prettier our whitespace should be pretty conventional so this shouldn't be a huuuge issue for our codebases).
I'd expect us to have a better AST here. There should be AST nodes for stuff like "this is a function with a name and these specific arguments" - you should be able to be able to say "only act when you find a function name, whose name is "spacing" and has these specific values as the first argument". We should leverage a tokeniser, rather than trying to fuzzy match ourselves. It looks like this behaviour of transforming values into "this is a function call" etc is provided by postcss-value-parser
stylelint has a demo of using postcss-value-parser https://github.com/stylelint/stylelint/blob/main/lib/rules/function-allowed-list/index.js#L36
polaris-migrator/src/migrations/replace-text-component/tests/replace-text-component.input.tsx
Show resolved
Hide resolved
|
Closing in favor of new iteration in #6852 |
Closes #6943 Adds the beginnings of the `@shopify/polaris-migrator` package. This package applies code transformations to help update Polaris apps and the Polaris codebase. ## Usage ```sh npx @shopify/polaris-migrator <migration> <path> ``` Run tests for `@shopify/polaris-migrator` ```sh yarn test --filter=@shopify/polaris-migrator ``` Original PR branch: #6701
2 participants
Adds the beginnings of the
@shopify/polaris-migratorpackage. This package applies code transformations to help updatePolaris apps and the Polaris codebase.
Usage
Run tests for
@shopify/polaris-migratoryarn test --filter=@shopify/polaris-migrator